Skip to content

Add draft for fixing (aria-)label bug on OcSelect #10823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

Add draft for fixing (aria-)label bug on OcSelect #10823

wants to merge 6 commits into from

Conversation

pascalwengerter
Copy link
Contributor

Description

From my understanding, the OcSelect component currently always gets a hardcoded "Search for option" aria-label, which overwrites the optional label that can be passed as a prop.
I've fixed that by only setting the "Search for option" aria-label if no label is passed via prop.
Another consideration is the visibility of the custom label, for which I added another prop - now, a custom label can be passed but also hidden in the GUI, while still available for screenreader users.

The next step for finalizing this PR (granted that my approach is approved) would be checking all occurances of the OcSelect and decide on whether they need a (hidden) custom label.

Related Issue

Motivation and Context

Accessibility improvements

@update-docs Update Docs
Copy link

update-docs bot commented Apr 23, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@dschmidt
Copy link
Member

While I usually like is and has prefixes I don't think they align with the props already present...

I'm not sure it's more elegant, but another approach might be to be able to pass in a label or an aria label... lf no aria label is passed, it uses the regular label. If only the aria label is passed, no label is shown.

No idea if that is a good or a bad thing but it allows to pass in two different values... and it avoids a boolean prop which is not super bad but always smells a bit to me for things like this

@pascalwengerter
Copy link
Contributor Author

While I usually like is and has prefixes I don't think they align with the props already present...

I'm not sure it's more elegant, but another approach might be to be able to pass in a label or an aria label... lf no aria label is passed, it uses the regular label. If only the aria label is passed, no label is shown.

No idea if that is a good or a bad thing but it allows to pass in two different values... and it avoids a boolean prop which is not super bad but always smells a bit to me for things like this

Good idea, let me get started on it!

@pascalwengerter
Copy link
Contributor Author

I've decided to switch the used vue-select library for vs-vue3-select, a fork of a fork that is maintained a little better (the bar is low, though), esp. regarding custom translations which we'll need for the next a11y audit.

@JammingBen
Copy link
Contributor

I think this has been superseded by #11396.

@JammingBen JammingBen closed this Sep 17, 2024
@pascalwengerter
Copy link
Contributor Author

I think this has been superseded by #11396.

True, thanks for the housekeeping!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[a11y] 9.2.5.3 Visible labeling part of the accessible name
3 participants
@pascalwengerter @dschmidt @JammingBen